-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement git refs #79
Implement git refs #79
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good start to me! I made some minor comments on the phrasing, but feel free to use or ignore those, as none are terribly important, esp given time limits.
Co-authored-by: Yevgeniy Brikman <[email protected]>
``` $ go build . && ./fetch --repo=https://github.com/pete0emerson/demo --ref=banana demo Downloading git reference "banana" of https://github.com/pete0emerson/demo ... Extracting files from <repo>/ to demo ... 2 files extracted Download and file extraction complete. ``` Note that this doesn't do any testing yet (booo). It also doesn't consider the parsing of the ref for a potential tag constraint.
This closes #73 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this LGTM! The docs are clear, the tests cases seem sufficient, and the implementation looks clean. I'm not a CODEOWNER on this repo and I haven't contributed to this project before, so I'll defer the approval to one of the CODEOWNERs.
My only open question though is this: it seems like--ref
supersedes --tag
, --branch
, and --commit
, so why keep those flags around?
The only reason for keeping the other flags around is for backwards compatibility. Once any uses of it are moved over to |
Looks good & easy to understand! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
This closes #73
This implements a
--ref
flag which can be used instead of--tag
,--branch
, or--commit
.